Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix app mail - Add main domain into hosts file #1780

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Josue-T
Copy link
Contributor

@Josue-T Josue-T commented Feb 22, 2024

The problem

Some apps like synapse need to send email but don't have option to allow invalid certificate. In this case the target server defined on the app must match with the certificate provided by postfix (or dovecot in reception case). But currently it's not possible as postfix (and dovecot) are configured to only accept app connexion on local IP (so 127.0.0.1 or ::1).

So for instance we have a server with main domain yolo.xyz.

In synapse to send email we have mainly 2 solutions:

  • smtp_server: "localhost" -> this case don't work because the postfix will provide a certificate for yolo.xyz but the SMTP client expect to receive a certificate for the configured domain, so localhost in this case.
  • smtp_server: "yolo.xyz" -> this case don't work too because the connection from SMTP client is made into the external interface (like eth0) and postfix don't allow app connection from external interface.

Solution

To solve this I thought about 2 potential solutions:

  1. Add on the hosts file the main domain so the SMTP client configured to point to yolo.xyz will resolve this to 127.0.0.1 and so the connection will be with a matching certificate to the configured domain. And on postfix side the connection will be on local interface so it will be happy.
  2. Allow app connection from external IP. So we can configure the SMTP client to point to yolo.xyz. The provided certificate will match with the configured domain. And if postfix is configured to allow external connection from app there will be no problem.

This PR implement the first solution which I think is the best one.

PR Status

Tested on my side and it work well.

How to test

Run regen-conf for dnsmasq and ping main domain and see that ping go directly to 127.0.0.1.

For apps which send mail (or receive) which need a valid TLS certificate the way to make it work is to pass a valid domain (and not localhost), so the target domain match with the provided certificate.
But postfix and dovecot refuse app authentication from external IP. So we need to force the request on local interface (with the public domain).
@Josue-T Josue-T requested a review from alexAubin February 22, 2024 21:42
@Josue-T Josue-T mentioned this pull request Feb 22, 2024
12 tasks
@OniriCorpe
Copy link
Member

humm yes, but what if an app uses the domain name where it is hosted? and not the ynh main domain?

i think we have to do the same thing for all the installed doamins on ynh

@Josue-T
Copy link
Contributor Author

Josue-T commented Feb 23, 2024

Well, we don't must use the app domain. We must use a domain which has a valid certificate.

By example:

main_domain: yolo.xyz
other domain:

  • toto.net
  • my.yolo.xyz
  • matrix.yolo.xyz

Let's say that synapse is installed on matrix.yolo.xyz. We can easily configure synapse to use yolo.xyz for SMTP server and it work.

@OniriCorpe
Copy link
Member

Yes, but it’s a bit less explicit for the user who receives the mail

@Josue-T
Copy link
Contributor Author

Josue-T commented Feb 23, 2024

Yes, but it’s a bit less explicit for the user who receives the mail

Well for users it won't change anything as the can still come from [email protected] (if I follow my last example). It's only for packager which will need to configure mail server to main domain.

We can also add all domain but I'm not sure that everybody will be happy to edit a lot the hosts file. And more modification (to the hosts file) might make this PR more complicated to merge.

@zamentur
Copy link
Member

I don't understand why it's not currently enough. When i ping my main domain i already have ::1 or 127.0.0.1 as answer (and this domain is not listed in /etc/hosts).

Are you sure dnsmasq is correctly configured on your test instance ?

Let's say that synapse is installed on matrix.yolo.xyz. We can easily configure synapse to use yolo.xyz for SMTP server and it work.

It works, but it probably brokes nextcloud as it's not configured to accept local request (by default, nextcloud consider request with local ip as potential security attack).

And if the main domain is changed ? This means package should implement hook mechanism to change that...

@Josue-T
Copy link
Contributor Author

Josue-T commented Feb 23, 2024

I don't understand why it's not currently enough. When i ping my main domain i already have ::1 or 127.0.0.1 as answer (and this domain is not listed in /etc/hosts).

Are you sure dnsmasq is correctly configured on your test instance ?

Investiguated and on my test intance I didn't had 127.0.0.1 in my resolv.conf. If it set to 127.0.0.1 it work. But if it's not set to this it won't work. But I think we can't expect that all yunohost instance have the /etc/resolv.conf set to 127.0.0.1.

Let's say that synapse is installed on matrix.yolo.xyz. We can easily configure synapse to use yolo.xyz for SMTP server and it work.

It works, but it probably brokes nextcloud as it's not configured to accept local request (by default, nextcloud consider request with local ip as potential security attack).

Don't really understand as on your setup as you said it won't change anything.

And if the main domain is changed ? This means package should implement hook mechanism to change that...

Well yes you are true.

So anyway for now let's leave it as it is.

@alexAubin
Copy link
Member

I think we can't expect that all yunohost instance have the /etc/resolv.conf set to 127.0.0.1.

The diagnosis checks precisely for this and complains about it if that's not the case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants